Conversation
sukvvon
requested review from
choi2021,
pumpkiinbell and
seungrodotlee
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #237 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 33 34 +1
Lines 836 853 +17
Branches 254 260 +6
=========================================
+ Hits 836 853 +17 🚀 New features to boost your workflow:
|
sukvvon
force-pushed
the
feat/add-useEventListener
branch
3 times, most recently
from
ac3523a to
0bc7710
Compare
sukvvon
force-pushed
the
feat/add-useEventListener
branch
from
0bc7710 to
9f1ed15
Compare
…ject element case
| * @example | ||
| * function ClickButton() { | ||
| * const buttonRef = useRef<HTMLButtonElement>(null); | ||
| * | ||
| * useEventListener('click', (event) => { | ||
| * console.log('Button clicked', event); | ||
| * }, buttonRef); | ||
| * | ||
| * return <button ref={buttonRef}>Click me</button>; | ||
| * } |
There was a problem hiding this comment.
Does this have any better use than just using the onClick function?
There was a problem hiding this comment.
This example isn’t meant to suggest that useEventListener is better than onClick — it’s just a simple way to demonstrate how the hook works with a ref.
There was a problem hiding this comment.
I guess it will useful when using 'CustomEvent'
sukvvon
force-pushed
the
feat/add-useEventListener
branch
from
5228b30 to
c4472a3
Compare
| * useEventListener('click', (event) => { | ||
| * console.log('Document clicked at coordinates', event.clientX, event.clientY); | ||
| * }, document); |
There was a problem hiding this comment.
This implementation does not appear to be safe in an SSR (Server-Side Rendering) environment. Directly referencing client-side global variables such as document seems risky. We should look for a way to ensure safe operation even in SSR environments.
For example, when you need to attach handlers to client-side global variables like document or window, instead of referencing them directly, you can pass them as strings like 'document' or 'window'. Then, inside a useEffect, you can safely access them conditionally. This approach can help ensure stability in SSR environments.
| * useEventListener('click', (event) => { | |
| * console.log('Document clicked at coordinates', event.clientX, event.clientY); | |
| * }, document); | |
| * useEventListener('click', (event) => { | |
| * console.log('Document clicked at coordinates', event.clientX, event.clientY); | |
| * }, | |
| * // ensure that `document` is only referenced on the client side. | |
| * 'document' | |
| * ); |
|
Hi @sukvvon, thank you for your contribution! We recently updated the project direction (#302, #329).
We're closing this PR accordingly. If you're interested, we've also launched Contributions in other areas are always welcome! Thank you for your understanding. |
|
Apologies for closing this right away — I should have given you time to respond. The code quality here is excellent, and I want to acknowledge the effort. The main concern is that That said, if you think there's a way to make this work as a platform-independent abstraction, or if you'd like to explore contributing it to |
5 participants
Overview
Checklist
yarn run fixto format and lint the code and docs?yarn run test:coverageto make sure there is no uncovered line?